network: add DetachedSignatureAvailableCheck by parona-source · Pull Request #771 · pkgcore/pkgcheck

parona-source · 2026-02-14T15:44:17Z

@thesamesam was interested in this

I couldn't fully follow precedent with the network tests. The check deals with multiple network requests in one go so just a mocking of the returnvalue wasn't enough.

thesamesam

Thanks a lot for this. It works well for me as well.

arthurzam

Some small changes I want, but looks really good.

I even want to run it global wise to know if it finds something

arthurzam · 2026-02-17T17:12:20Z

src/pkgcheck/checks/network.py

+            for url in f.uri:
+                for extension in self.detached_signature_extensions:
+                    yield (f.filename, f"{url}{extension}")
+        return []


Suggested change

return []

no need for this return

This is also done this way in PyPIAttestationAvailableCheck. Should that be changed as well?

I mostly copied that check.

Applied suggestion. Didn't touch PyPIAttestationAvailableCheck

Edit: Touched it by minimising it as well.

src/pkgcheck/checks/network.py

arthurzam · 2026-02-17T17:15:09Z

src/pkgcheck/checks/network.py

+        result = future.result()
+        if result is not None:


Suggested change

result = future.result()

if result is not None:

if (result := future.result()):

_UrlCheck and PypiAttestationAvailableCheck do this as well. Should I touch these?

Realised I could minimise duplicated code by using _UrlCheck. Didn't touch them but I removed this from the new check.

PypiAttestationAvailableCheck looks like it could get minimised as well.

Edit: Touched them.

src/pkgcheck/checks/network.py

Signed-off-by: Alfred Wingate <parona@protonmail.com>

ferringb · 2026-02-18T19:07:56Z

The code looks fine- these are just potential points of polish or questions about being brutally paranoid about robustness ;)

I couldn't fully follow precedent with the network tests. The check deals with multiple network requests in one go so just a mocking of the returnvalue wasn't enough.

I don't know that code in question, but that sounds like something our test utilities should be enhanced to support. Mind cutting a ticket laying out what you think it should be? IE, what would've allowed you to use it rather than having to sidestep it?

ferringb · 2026-02-18T18:52:59Z

src/pkgcheck/checks/network.py


-        result = future.result()
-        if result is not None:
+        if result := future.result():


Is there any scenario where result can validly be not bool(future.result()) for a returned result?

In terms of actual 'api' safety, a check of future.done() is safer, but it's also a more obnoxious chain of calls..

I mention it in reading the code; this is neither a "must" nor a "should"; just an observation since this is being converted away from is None

ferringb · 2026-02-18T18:54:11Z

src/pkgcheck/checks/network.py



-class PyPIAttestationAvailableCheck(NetworkCheck):
+class PyPIAttestationAvailableCheck(_UrlCheck):


unrelated to your change, but we should promote _UrlCheck to a public base, albeit after lacing ABC through it.

ferringb · 2026-02-18T18:56:42Z

src/pkgcheck/checks/network.py

+            self._schedule_check(self._provenance_check, filename, url, executor, futures, pkg=pkg)
+
+
+class DetachedSignatureAvailable(results.VersionResult, results.Info):


Just FYI- none of this inheritance chain currently sets __slots__, but they'll be converted over. That's both to prevent accidental access, and for serialization speed reasons..

No change needed here, again, just FYI.

ferringb · 2026-02-18T19:03:56Z

src/pkgcheck/checks/network.py

+
+    def _verifysig_check(self, filename, url, *, pkg):
+        """Check for typical verify sig URLS."""
+        result = None


Thoughts on converting this flow to just be returns? Your code is fine- there's no possible uninitialized scenario, but if you just lace returns in, the possibility goes away in full while simplifying the control flow considerations.

IE.

try: # Need redirects to deal with the variance of file servers and urls response = self.session.head(url, allow_redirects=True) except RequestError: # should this be debug logged, or does something further down the chain log it? return None except SSLError as e: return SSLCertificateError("SRC_URI", url, str(e), pkg=pkg) content_type = response.headers.get("Content-Type") # Filtering out text/html matches is useful due to possible false matches with authentication if ( response.ok and content_type is not None and not content_type.startswith("text/html") ): return DetachedSignatureAvailable(filename, url, pkg=pkg) return None

That's just an observation of a possible improvement.

Actually, one tweak here simplifies things:

# Filtering out text/html matches is useful due to possible false matches with authentication if ( response.ok and not response.headers.get("Content-Type", "").startswith("text/html") ):

You can skip the intermediate content_type

parona-source · 2026-02-18T19:23:08Z

I couldn't fully follow precedent with the network tests. The check deals with multiple network requests in one go so just a mocking of the returnvalue wasn't enough.

I don't know that code in question, but that sounds like something our test utilities should be enhanced to support. Mind cutting a ticket laying out what you think it should be? IE, what would've allowed you to use it rather than having to sidestep it?

I meant that with other network tests are simpler with having only one request triggered. So instead of simply setting requests = ... etc in responses.py, I had to have a function where I kept the implementation details for Session in mind. (Which is where @contextmanager and yield come from).

Example:
https://github.com/pkgcore/pkgcheck/blob/master/testdata/repos/network/MetadataUrlCheck/HttpsUrlAvailable/responses.py

Session:
https://github.com/pkgcore/pkgcheck/blob/master/src/pkgcheck/addons/net.py#L36

I don't see an issue with it as is, it currently means there is some ready made boilerplate for the next time its needed.

parona-source force-pushed the detached-signature-check branch from 8697f61 to 694a268 Compare February 14, 2026 15:50

thesamesam approved these changes Feb 17, 2026

View reviewed changes

arthurzam requested changes Feb 17, 2026

View reviewed changes

parona-source force-pushed the detached-signature-check branch from 694a268 to 4269370 Compare February 18, 2026 11:53

network: add DetachedSignatureAvailableCheck

9f22eac

Signed-off-by: Alfred Wingate <parona@protonmail.com>

parona-source force-pushed the detached-signature-check branch from 4269370 to 9f22eac Compare February 18, 2026 11:55

parona-source added 2 commits February 18, 2026 13:58

network: less boilerplate by using _UrlCheck

7c5a43f

Signed-off-by: Alfred Wingate <parona@protonmail.com>

network: use the walrus operator where applicable

b8f8b4b

Signed-off-by: Alfred Wingate <parona@protonmail.com>

parona-source force-pushed the detached-signature-check branch from 4ca63cf to b8f8b4b Compare February 18, 2026 12:12

parona-source requested a review from arthurzam February 18, 2026 12:16

ferringb reviewed Feb 18, 2026

View reviewed changes

	result = future.result()
	if result is not None:
	if (result := future.result()):



		class PyPIAttestationAvailableCheck(NetworkCheck):
		class PyPIAttestationAvailableCheck(_UrlCheck):

		self._schedule_check(self._provenance_check, filename, url, executor, futures, pkg=pkg)


		class DetachedSignatureAvailable(results.VersionResult, results.Info):

Comments

Conversation

parona-source commented Feb 14, 2026

Uh oh!

thesamesam left a comment

Choose a reason for hiding this comment

Uh oh!

arthurzam left a comment

Choose a reason for hiding this comment

Uh oh!

arthurzam Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

parona-source Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

parona-source Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arthurzam Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

parona-source Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

parona-source Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ferringb commented Feb 18, 2026

Uh oh!

ferringb Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferringb Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

ferringb Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

ferringb Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ferringb Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

parona-source commented Feb 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

parona-source Feb 18, 2026 •

edited

Loading

parona-source Feb 18, 2026 •

edited

Loading

parona-source Feb 18, 2026 •

edited

Loading

ferringb Feb 18, 2026 •

edited

Loading

ferringb Feb 18, 2026 •

edited

Loading